Skip to content

Feature: McpTransportContext for HttpServletSseServerTransportProvider #477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Feature: McpTransportContext for HttpServletSseServerTransportProvider #477

wants to merge 12 commits into from
+163 −12

Conversation

stantonk
Copy link

@stantonk stantonk commented Aug 10, 2025 •
edited
Loading

Implements #476.

Motivation and Context

#420 introduced McpTransportContext.

McpTransportContext should be supported in HttpServletSseServerTransportProvider to enable those who are still on that transport to leverage McpTransportContext to set attributes that Tools, etc. have access to. This is crucial for implementing Authentication & Authorization at the Tool level.

Current Behavior

McpTransportContext is absent from HttpServletSseServerTransportProvider

Context

How has this issue affected you?

We're not able to switch to HttpServletStreamableServerTransportProvider yet, but wish to use McpTransportContext instead of our custom built similar capability.

What are you trying to accomplish?

Authn/z at Tool level.

How Has This Been Tested?

  • In a langchain4j agent application calling into a Dropwizard application serving MCP using HttpServletSseServerTransportProvider
  • directly by testing a Dropwizard application serving MCP using HttpServletSseServerTransportProvider
  • Integration tests included with PR

Breaking Changes

No, use of the McpTransportContext is optional.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

None

This was referenced Aug 10, 2025
Merged
Open
@stantonk stantonk changed the title Feature/mcp transport context for http servlet sse server transport provider Feature: McpTransportContext for HttpServletSseServerTransportProvider Aug 10, 2025
@tzolov tzolov requested a review from chemicL August 10, 2025 09:08
This was referenced Aug 12, 2025
Closed
Closed
Merged
chemicL
chemicL requested changes Aug 13, 2025
View reviewed changes
Copy link
Member

@chemicL chemicL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I think this is useful and seems you found a way around the singleton behaviour of the exchange object :) Left some comments to improve a few things, but I agree we should merge this once fixed. Also, would you consider following up with the Spring-related SSE transport providers to maintain feature parity?

@chemicL chemicL mentioned this pull request Aug 13, 2025
Open
@stantonk
Copy link
Author

@chemicL thanks for the quick review! All your suggestions make sense, I'll get those updates made.

RE: Spring support, I've never used Spring but I can take a pass at it, probably this weekend. If I hit some snags would you be open to merging the Servlet version in first? I just don't want to stay on our internal fork for too long as additional capabilities and bug fixes get merged in.

stantonk and others added 6 commits August 17, 2025 16:57
@stantonk
make new constructor private
a8de52a
@stantonk
specify no-op McpTransportContextExtractor in deprecated constructors…
03057e4 
…; add notNull assertions
@stantonk
add this to contextExtractor instance variable reference
f88a697
@stantonk
add comment explaining creation of McpAsyncServerExchange copy for ea…
00b092d 
…ch request in handleIncomingRequest and handleIncomingNotification
@stantonk
use AtomicBooleans to capture state of transportContext and responseB…
527643e 
…ody to peform assertions within JUnit test thread; since callHandler may run on a different thread
@stantonk @chemicL
Update mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpCl…
add21cd 
…ientServerIntegrationTests.java

Co-authored-by: Dariusz Jędrzejczyk <[email protected]>
@stantonk stantonk requested a review from chemicL August 17, 2025 23:13
@stantonk
Copy link
Author

@chemicL thanks again for the review!

Updates

  • I've made all your recommended updates

Testing

  • I re-ran mvn clean verify locally and all tests passed
  • I ran mvn clean install locally, and configured our Dropwizard MCP Service to use the latest HEAD of this PR after all your requested changes. I confirmed our AI Agent service talking to our Dropwizard MCP Service is functioning properly with the latest changes, including our Authentication & Authorization into the individual MCP Tool call level via the McpTransportContextExtractor using HttpServletSseServerTransportProvider. Initialization, list tools, and tool calls are working properly.

Spring WebMvc and WebFlux

I took a quick look through the code, it all looks pretty similar so I should be able to follow up in a subsequent PR adding McpTransportContext to those as well. I've got a busy week coming up, but I'll try for next weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chemicL chemicL Awaiting requested review from chemicL

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stantonk @chemicL